Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding iterators #83

Merged
merged 2 commits into from
Jul 25, 2023
Merged

adding iterators #83

merged 2 commits into from
Jul 25, 2023

Conversation

dlockhart
Copy link
Member

@dlockhart dlockhart commented Jul 20, 2023

You can now iterate between test files or tests within files. The "Home" button is now a breadcrumb on the left, and I made a few other minor stylistic tweaks.

Screenshot 2023-07-20 at 4 58 02 PM

@dlockhart dlockhart requested a review from a team as a code owner July 20, 2023 20:58
grid-template-columns: 300px auto;
grid-template-columns: 275px auto;
height: 100vh;
overflow: hidden;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was able to simplify the CSS grid stuff a bit.

padding-top: 20px;
}
.list-file-title {
padding-bottom: 20px;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test list now uses similar spacing and separators as the results:

Screenshot 2023-07-20 at 4 59 30 PM

`;
}
_handleBackClick() {
this._updateSearchParams({ file: undefined, test: undefined });
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and _handleListTestClick didn't end up being necessary due to the <a href> doing the same thing -- it was actually firing things twice.

@@ -312,6 +306,7 @@ class App extends LitElement {
return renderTabButtons('browser results', tabs, index => {
this._selectedBrowserIndex = index;
this.shadowRoot.querySelector('.tab-panels').scrollTo(0, 0);
this.requestUpdate();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_selectedBrowserIndex isn't a reactive state property anymore since we modify it ourselves. But in this case they've selected a new tab so we force a re-render.

this._selectedBrowserIndex = Math.max(
browsers.findIndex((b) => browserResults.get(b.name) < tests.length),
0
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to remember the index we automatically pick for them so that it stays as they iterate.

Copy link
Contributor

@svanherk svanherk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those chevrons are huge lol!

@dlockhart
Copy link
Member Author

Those chevrons are huge lol!

Haha I know... they're our "tier 2" chevrons but could maybe use some slimming eh.

@dlockhart
Copy link
Member Author

Ok I've switched to an upscaled version of our tier1 chevrons instead, which are slimmer:
Screenshot 2023-07-24 at 4 22 08 PM

I'm sure Jeff will want to tweak these anyway, so don't want to spend too much time on them.

@svanherk
Copy link
Contributor

I'm sure Jeff will want to tweak these anyway, so don't want to spend too much time on them.

Haha yeah I wasn't too worried about it for that reason, but they do look much better!

@dlockhart dlockhart merged commit 1777111 into main Jul 25, 2023
@dlockhart dlockhart deleted the dlockhart/iterators branch July 25, 2023 13:32
@ghost
Copy link

ghost commented Jul 25, 2023

🎉 This PR is included in version 0.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants